Skip to content

[dotnet] [bidi] Remove unnecessary command type info #16454

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Oct 17, 2025
Merged

[dotnet] [bidi] Remove unnecessary command type info #16454

merged 1 commit into from
Oct 17, 2025
+2 −6

Conversation

nvborisenko
Copy link
Member

@nvborisenko nvborisenko commented Oct 17, 2025 •
edited by qodo-merge-pro bot
Loading

User description

💥 What does this PR do?

Clean command type

🔄 Types of changes

  • Cleanup (formatting, renaming)

PR Type

Enhancement

Description

  • Remove unnecessary ResultType property from Command class

  • Simplify constructor by removing resultType parameter

  • Update generic Command<TParameters, TResult> to match new signature

  • Reduce code complexity and improve maintainability

Diagram Walkthrough

flowchart LR
  A["Command class<br/>with ResultType property"] -- "Remove ResultType<br/>and constructor param" --> B["Simplified Command class<br/>with only Method and Id"]
  C["Command&lt;TParameters, TResult&gt;<br/>passes typeof(TResult)"] -- "Update to match<br/>new signature" --> D["Command&lt;TParameters, TResult&gt;<br/>simplified constructor call"]
Loading

File Walkthrough
Relevant files
Cleanup
Command.cs
Remove ResultType property and simplify Command class       

dotnet/src/webdriver/BiDi/Communication/Command.cs

  • Removed resultType parameter from Command constructor
  • Deleted ResultType property and its [JsonIgnore] attribute
  • Updated Command base class call to remove
    typeof(TResult) argument
  • Streamlined class design by removing unused type information
 +2/-6     

@qodo-merge-pro Qodo Merge Pro
Copy link
Contributor

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪
🎫 No ticket provided 
- [ ] Create ticket/issue <!-- /create_ticket --create_ticket=true -->

</details></td></tr>
Codebase Duplication Compliance
⚪
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
⚪
No custom compliance provided

Follow the guide to enable custom compliance check.
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@selenium-ci selenium-ci added the C-dotnet .NET Bindings label Oct 17, 2025
@qodo-merge-pro Qodo Merge Pro
Copy link
Contributor

PR Code Suggestions ✨

No code suggestions found for the PR.

@nvborisenko nvborisenko merged commit e71fa0a into SeleniumHQ:trunk Oct 17, 2025
13 checks passed
@nvborisenko nvborisenko deleted the bidi-remove-command-res-type branch October 17, 2025 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@VietND96 VietND96 VietND96 approved these changes

+1 more reviewer

@asolntsev asolntsev asolntsev approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

C-dotnet .NET Bindings Review effort 1/5

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@nvborisenko @asolntsev @VietND96 @selenium-ci